screenshot: Blocklist the current screencast pipeline if recorder failed

When gstreamer crashes during recording, it pulls the whole screencastService
down with it.

These crashes are typically caused by the gstreamer pipeline that's in use,
so to avoid running into them again and again, we can blocklist the last
used pipeline in case the recorder didn't shut down (aka crashed) last time.

To store this state, create a file (gnome-shell-screencast-pipeline-blocklist)
in the XDG runtime dir and store the ID of the current pipeline in that file
before we try to start.

Now when we crash while running the pipeline, the entry in that file will stay
around and we'll pick it up on the next start of the screencastService as a
blocklist.

When the recording was successful on the other hand, we'll call
`this._updateServiceCrashBlocklist([...this._blocklistFromPreviousCrashes])`
and remove the new entry from the file again before shutting down the recorder.

In addition to that, we can now encourage the user to try recording again
after a crash happened. Adjust the failure notification a bit to say
"please try again".

https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/6747

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2976>

diff --git a/js/dbusServices/screencast/screencastService.js b/js/dbusServices/screencast/screencastService.js
index 9a447942499ea655bf378a53a42fce85b5fc90b9..ec136c91ddcb559e5be7887f8225eaf72e916cdb 100644 (file)
--- a/js/dbusServices/screencast/screencastService.js
+++ b/js/dbusServices/screencast/screencastService.js
@@ -28,8 +28,11 @@ const ScreenCastStreamProxy = Gio.DBusProxy.makeProxyWrapper(ScreenCastStreamIfa
 const DEFAULT_FRAMERATE = 30;
 const DEFAULT_DRAW_CURSOR = true;
 
+const PIPELINE_BLOCKLIST_FILENAME = 'gnome-shell-screencast-pipeline-blocklist';
+
 const PIPELINES = [
     {
+        id: 'swenc-dmabuf-vp8-vp8enc',
         pipelineString:
             'capsfilter caps=video/x-raw(memory:DMABuf),max-framerate=%F/1 ! \
              glupload ! glcolorconvert ! gldownload ! \
@@ -39,6 +42,7 @@ const PIPELINES = [
              webmmux',
     },
     {
+        id: 'swenc-memfd-vp8-vp8enc',
         pipelineString:
             'capsfilter caps=video/x-raw,max-framerate=%F/1 ! \
              videoconvert chroma-mode=none dither=none matrix-mode=output-only n-threads=%T ! \
@@ -90,6 +94,20 @@ class Recorder extends Signals.EventEmitter {
         this._pipelineString = null;
         this._framerate = DEFAULT_FRAMERATE;
         this._drawCursor = DEFAULT_DRAW_CURSOR;
+        this._blocklistFromPreviousCrashes = [];
+
+        const pipelineBlocklistPath = GLib.build_filenamev(
+            [GLib.get_user_runtime_dir(), PIPELINE_BLOCKLIST_FILENAME]);
+        this._pipelineBlocklistFile = Gio.File.new_for_path(pipelineBlocklistPath);
+
+        try {
+            const [success_, contents] = this._pipelineBlocklistFile.load_contents(null);
+            const decoder = new TextDecoder();
+            this._blocklistFromPreviousCrashes = JSON.parse(decoder.decode(contents));
+        } catch (e) {
+            if (!e.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.NOT_FOUND))
+                throw e;
+        }
 
         this._pipelineState = PipelineState.INIT;
         this._pipeline = null;
@@ -154,6 +172,12 @@ class Recorder extends Signals.EventEmitter {
     _bailOutOnError(message, errorDomain = ScreencastErrors, errorCode = ScreencastError.RECORDER_ERROR) {
         const error = new GLib.Error(errorDomain, errorCode, message);
 
+        // If it's a PIPELINE_ERROR, we want to leave the failing pipeline on the
+        // blocklist for the next time. Other errors are pipeline-independent, so
+        // reset the blocklist to allow the pipeline to be tried again next time.
+        if (!error.matches(ScreencastErrors, ScreencastError.PIPELINE_ERROR))
+            this._updateServiceCrashBlocklist([...this._blocklistFromPreviousCrashes]);
+
         this._teardownPipeline();
         this._unwatchSender();
         this._stopSession();
@@ -195,6 +219,20 @@ class Recorder extends Signals.EventEmitter {
         this._sessionProxy.connectSignal('Closed', this._onSessionClosed.bind(this));
     }
 
+    _updateServiceCrashBlocklist(blocklist) {
+        try {
+            if (blocklist.length === 0) {
+                this._pipelineBlocklistFile.delete(null);
+            } else {
+                this._pipelineBlocklistFile.replace_contents(
+                    JSON.stringify(blocklist), null, false,
+                    Gio.FileCreateFlags.NONE, null);
+            }
+        } catch (e) {
+            console.log(`Failed to update pipeline-blocklist file: ${e.message}`);
+        }
+    }
+
     _tryNextPipeline() {
         const {done, value: pipelineConfig} = this._pipelineConfigs.next();
         if (done) {
@@ -203,6 +241,12 @@ class Recorder extends Signals.EventEmitter {
             return;
         }
 
+        if (this._blocklistFromPreviousCrashes.includes(pipelineConfig.id)) {
+            console.info(`Skipping pipeline '${pipelineConfig.id}' due to pipeline blocklist`);
+            this._tryNextPipeline();
+            return;
+        }
+
         if (this._pipeline) {
             if (this._pipeline.set_state(Gst.State.NULL) !== Gst.StateChangeReturn.SUCCESS)
                 log('Failed to set pipeline state to NULL');
@@ -213,12 +257,13 @@ class Recorder extends Signals.EventEmitter {
         try {
             this._pipeline = this._createPipeline(this._nodeId, pipelineConfig,
                 this._framerate);
-        } catch (error) {
-            this._tryNextPipeline();
-            return;
-        }
 
-        if (!this._pipeline) {
+            // Add the current pipeline to the blocklist, so it is skipped next
+            // time in case we crash; we'll remove it again on success or on
+            // non-pipeline-related failures.
+            this._updateServiceCrashBlocklist(
+                [...this._blocklistFromPreviousCrashes, pipelineConfig.id]);
+        } catch (error) {
             this._tryNextPipeline();
             return;
         }
@@ -332,6 +377,10 @@ class Recorder extends Signals.EventEmitter {
                 break;
 
             case PipelineState.FLUSHING:
+                // The pipeline ran successfully and we didn't crash; we can remove it
+                // from the blocklist again now.
+                this._updateServiceCrashBlocklist([...this._blocklistFromPreviousCrashes]);
+
                 this._addRecentItem();
 
                 this._teardownPipeline();

diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index fecc4584d822b7cf7306888beda008546d98aec5..eb561a3b059d67832a9b443c6a42256a7b872ec2 100644 (file)
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -2044,11 +2044,17 @@ export const ScreenshotUI = GObject.registerClass({
             if (error.matches(ScreencastErrors, ScreencastError.OUT_OF_DISK_SPACE)) {
                 // Translators: notification title.
                 this._showNotification(_('Screencast ended: Out of disk space'));
-                return;
+            } else if (error.matches(ScreencastErrors, ScreencastError.SERVICE_CRASH)) {
+                // We can encourage user to try again on service crashes since the
+                // recorder will auto-blocklist the pipeline that crashed.
+
+                // Translators: notification title.
+                this._showNotification(_('Screencast ended unexpectedly, please try again'));
+            } else {
+                // Translators: notification title.
+                this._showNotification(_('Screencast ended unexpectedly'));
             }
 
-            // Translators: notification title.
-            this._showNotification(_('Screencast ended unexpectedly'));
             break;
         }
     }